Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrating acs plugin #2956

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

migrating acs plugin #2956

wants to merge 7 commits into from

Conversation

maknop
Copy link

@maknop maknop commented Feb 19, 2025

Hey, I just made a Pull Request!

The purpose of this PR is to add a new upstream plugin for Red Hat's Advanced Cluster Security. This PR displays vulnerability data for entities in the catalog. This frontend plugin queries the ACS API by the deployment name to retrieve and display vulnerability data.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

acs_plugin_screenshot_1
acs_plugin_screenshot_2

Signed-off-by: Matt Knop <[email protected]>
@maknop maknop requested review from backstage-service and a team as code owners February 19, 2025 23:38
@maknop maknop requested a review from BethGriggs February 19, 2025 23:38
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Feb 19, 2025

Changed Packages

Package Name Package Path Changeset Bump Current Version
app workspaces/acs/packages/app none v0.0.0
backend workspaces/acs/packages/backend none v0.0.0
@backstage-community/plugin-acs workspaces/acs/plugins/acs none v0.0.0

Signed-off-by: Matt Knop <[email protected]>
@maknop
Copy link
Author

maknop commented Feb 19, 2025

@awanlin here is the new PR for the ACS plugin. You mentioned to ping you when I reopened it since the last one got closed.

Here is the reference to the old one 😄 #2638

@awanlin awanlin requested review from awanlin and removed request for BethGriggs February 20, 2025 15:27
@awanlin
Copy link
Contributor

awanlin commented Feb 20, 2025

Hi @BethGriggs, I reviewed the previous PR so I'll take this one 👍

@awanlin
Copy link
Contributor

awanlin commented Feb 20, 2025

Hi @maknop, I've triggers the build and it's failed trying to to the yarn install can you look into that please?

@maknop
Copy link
Author

maknop commented Feb 20, 2025

Hi @maknop, I've triggers the build and it's failed trying to to the yarn install can you look into that please?

Yes I'll take a look now.

@maknop
Copy link
Author

maknop commented Feb 20, 2025

@awanlin where do you see that failure? I wanted to look at the build failure so I can see the logs.

@awanlin
Copy link
Contributor

awanlin commented Feb 21, 2025

@awanlin where do you see that failure? I wanted to look at the build failure so I can see the logs.

Do you not see this? I'm asking just incase there some issue preventing you from seeing it. You should be able to click on those failed items to see more details.

image

Here a link to the failure: https://github.com/backstage/community-plugins/actions/runs/13441006388/job/37574597968?pr=2956#step:5:21

@maknop
Copy link
Author

maknop commented Feb 21, 2025

I don't know why but I couldn't see the test failures until now. I checked multiple times today. I can see them now though so I'll see what the issue is. Thanks!

@maknop
Copy link
Author

maknop commented Feb 21, 2025

So the issue is that it is trying to install @backstage-community/plugin-acs but that doesn't exist yet.. my understanding was that getting that setup was part of the process. Am I missing something there?

@maknop
Copy link
Author

maknop commented Feb 21, 2025

I updated the branch and now it's just stuck on this. This is what I was seeing earlier when I didn't see the results of the pipeline runs.
image

@awanlin
Copy link
Contributor

awanlin commented Feb 21, 2025

As you have not had anything merged into this repo yet it won't run the CI, each time you push a commit it resets that. Part of the issue is that it looks like you keep updating from main which would also reset the CI.

@maknop
Copy link
Author

maknop commented Feb 21, 2025

Okay, I see. So the issue is that it is trying to install @backstage-community/plugin-acs but that doesn't exist yet.. my understanding was that getting that setup was part of the process. Am I missing something there @awanlin?

@maknop
Copy link
Author

maknop commented Feb 21, 2025

Actually I think I see the issue. This is in the plugins/acs/package.json:

"backstage": {
    "role": "frontend-plugin",
    "pluginId": "acs",
    "pluginPackages": [
      "@backstage-community/plugin-acs"
    ]
  },

I should remove this I believe. This NPM package doesn't exist yet. I removed this block of code in the package.json and moved the images dir and README.md within the acs plugin dir since I realized it was in the wrong spot 😄

@maknop
Copy link
Author

maknop commented Feb 21, 2025

Cool, just pushed up a fix 😄

@backstage-goalie
Copy link
Contributor

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

Signed-off-by: Matt Knop <[email protected]>
@awanlin
Copy link
Contributor

awanlin commented Feb 23, 2025

Hi @maknop, as there are still some CI errors to address that will impact the code base I'm going to wait on reviewing until it's green. I will keep an eye on this to approve the build so you aren't stuck in the mean time 👍

@maknop
Copy link
Author

maknop commented Feb 23, 2025

Hey @awanlin, I'll go ahead and work through those tomorrow. What all goes into the review once the CI errors are all resolved and everything is green?

@maknop
Copy link
Author

maknop commented Feb 24, 2025

One thing I don't really understand is that there are errors in the CI check that weren't from code I specifically wrote but that were populated from the plugin bootstrap command. I basically moved the code of the entire plugin from a different repo that was being worked on into here.

I'm working through the fixes on the code I wrote but this plugin does come up as it should when I run a yarn dev from the workspace root.

@maknop
Copy link
Author

maknop commented Feb 24, 2025

@awanlin Could I get another rerun when you have a moment :)

@maknop
Copy link
Author

maknop commented Feb 25, 2025

@awanlin Hi! I have progressed further on the CI errors. Would you mind kicking it off again for me please? 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants